Reform internal use of template compiler #20587

ef4 · 2023-12-01T18:51:23Z

Previously, ember-source had a weird custom way of compiling templates that appear within its own source. This PR makes it work the same way we do everywhere else, using babel-plugin-ember-template-compilation.

This required solving a boostrapping problem: the babel plugin needs ember-template-compiler.js, our build produces
ember-template-compiler.js, but our build wants to use the babel plugin. I solved it by making the template compiler directly evaluatable in node, with no build at all.

The full legacy ember-template-compiler API has a lot of silly things shoved into it (like a reexport of all of the Ember metapackage), and that was inextricably circular, so I introduced ember-template-compiler/minimal for this internal use case.

ef4 · 2023-12-01T18:52:23Z

broccoli/packages.js

@@ -54,32 +53,21 @@ module.exports.qunit = function _qunit() {

 module.exports.getPackagesES = function getPackagesES() {
  let input = new Funnel(`packages`, {
-    exclude: ['loader/**', 'external-helpers/**'],
+    exclude: ['loader/**', 'external-helpers/**', '**/node_modules'],


This PR makes all the packages real pnpm workspaces, which was necessary to make the ember-template-compiler package actually evaluatable in node. Previously not all packages were. This extra filtering is necessary now that pnpm is creating node_modules directories inside the packages.

ef4 · 2023-12-01T18:53:24Z

ember-cli-build.js

+      ],
+      {
+        // we're replacing the ember/verion file with the actual version number
+        overwrite: true,


The template compiler imports the version. Previously that would die because the version file is dynamically created. Now we have a placeholder file on disk that gets replaced in the build, so that our internal usage of template compiler doesn't die.

ef4 · 2023-12-01T18:54:12Z

lib/index.js

+      compilerPath = path.resolve(__dirname, '../broccoli/glimmer-template-compiler');
+    } else {
+      // When we're building an app, we use the already built template compiler.
+      compilerPath = path.resolve(__dirname, '../dist/ember-template-compiler.js');


This keeps the exact old behavior when building apps, and only changes the behavior when ember is building itself.

ef4 · 2023-12-01T18:54:31Z

package.json

@@ -111,6 +111,8 @@
    "@embroider/shared-internals": "^2.5.0",
    "@rollup/plugin-babel": "^6.0.3",
    "@simple-dom/document": "^1.4.0",
+    "@swc/core": "^1.3.100",
+    "@swc-node/register": "^1.6.8",


This is how we can directly evaluate the template compiler without a build.

ef4 · 2023-12-01T18:55:05Z

packages/@ember/-internals/glimmer/lib/templates/empty.ts

+import { precompileTemplate } from '@ember/template-compilation';
+export default precompileTemplate('', {
+  moduleName: 'packages/@ember/-internals/glimmer/lib/templates/empty.hbs',
+});


I was able to drop all special support for hbs files, and several hacks around their types, by making them just be the equivalent javascript.

ef4 · 2023-12-01T18:55:57Z

packages/ember/version.ts

@@ -0,0 +1,2 @@
+// this file gets replaced with the real value during the build
+export default 'VERSION_GOES_HERE';


By having a real file on disk, we get to eliminate hacks around the types and we avoid crashing if you try to directly evaluate in node, without a build.

ef4 · 2023-12-01T18:56:24Z

pnpm-workspace.yaml

+packages:
+ - 'packages/*'
+ - 'packages/@ember/*'
+ - 'packages/@glimmer/*'


When I converted this repo to pnpm, it looks like pnpm was auto-detecting all our packages. But it turns out it was not detecting all of them. This makes it explicit.

ef4 · 2023-12-01T18:57:18Z

smoke-tests/ember-test-app/pnpm-workspace.yaml

@@ -0,0 +1,2 @@
+# this tells pnpm we are a separate project with a separate install, not part of
+# the monorepo's workspaces.


Adding pnpm-workspace.yaml at the root required also adding this, in order to keep the old behavior where this subdir gets an entirely independent pnpm install.

Arguably it would be better to just make this subdir another workspace, but that turns out to be non-trivial given NPM dependency hell, so I didn't try to attempt that change in this PR.

ef4 · 2023-12-01T18:57:40Z

types/publish.mjs

-  '@ember/-internals/glimmer/lib/templates/outlet.d.ts',
-  '@ember/-internals/glimmer/lib/templates/root.d.ts',
-  '@ember/-internals/glimmer/lib/templates/textarea.d.ts',
-];


All of this was helping typescript deal with fake files that don't exist. But now all of them are real files.

ef4 · 2023-12-01T19:03:19Z

packages/ember/version.d.ts

@@ -1,2 +0,0 @@
-declare const VERSION: string;


This type was actually a lie. The generated version module only contains a default export. So this PR is also a bugfix to the types.

ef4 · 2023-12-01T20:04:52Z

Oh wow, GitHub automatically changed the base branch after merging the other one, nice.

Previously, ember-source had a weird custom way of compiling templates that appear within its own source. This PR makes it work the same way we do everywhere else, using babele-plugin-ember-template-compilation. This required solving a boostrapping problem: the babel plugin needs ember-template-compiler.js, our build produces ember-template-compiler.js, but our build wants to use the babel plugin. I solved it by making the template compiler directly evaluatable in node, with no build at all. The *full* legacy ember-template-compiler API has a lot of silly things shoved into it (like a reexport of all of the `Ember` metapackage), and that was inextricably circular, so I introduced `ember-template-compiler/minimal` for this internal use case.

NullVoxPopuli

Super excited for this! thanks for the self review, that was super helpful!

As of #20587, the modules published under `dist/packages` act more like normal addon code, in that their templates are published as calls to `precompileTemplate` rather than wire format. But that makes this the first time that Embroider is seeing these templates, and because they are in loose mode and use the dynamic component helper they are un-analyzable. This PR: - switches them to strict mode - fixes the declared types for `precompileTemplate`, which were a lie before. `scope` is allowed in non-strict mode. `scope` is optional even in strict mode. - fixes a spelling error in a recently-introduced internal method, because I noticed it and it annoyed me.

…ncies This is a bugfix to #20587. We need to have an explicit dependency (not devDependency) on babel-plugin-ember-template-compilation because it's used from our addon main, which runs inside apps builds.

As of #20587, the modules published under `dist/packages` act more like normal addon code, in that their templates are published as calls to `precompileTemplate` rather than wire format. But that makes this the first time that Embroider is seeing these templates, and because they are in loose mode and use the dynamic component helper they are un-analyzable. This PR: - switches them to strict mode - fixes the declared types for `precompileTemplate`, which were a lie before. `scope` is allowed in non-strict mode. `scope` is optional even in strict mode. - fixes a spelling error in a recently-introduced internal method, because I noticed it and it annoyed me. (cherry picked from commit 82b5b1d)

…ncies This is a bugfix to #20587. We need to have an explicit dependency (not devDependency) on babel-plugin-ember-template-compilation because it's used from our addon main, which runs inside apps builds. (cherry picked from commit 6d33841)

ef4 commented Dec 1, 2023

View reviewed changes

ef4 force-pushed the direct-eval-template-compiler branch from c3afc77 to 2efb6a9 Compare December 1, 2023 19:02

ef4 commented Dec 1, 2023

View reviewed changes

Base automatically changed from internal-coordination to main December 1, 2023 19:08

ef4 force-pushed the direct-eval-template-compiler branch from 2efb6a9 to 46c0784 Compare December 1, 2023 20:04

ef4 force-pushed the direct-eval-template-compiler branch from 3b99f94 to b4ab87b Compare December 2, 2023 21:02

ef4 marked this pull request as ready for review December 2, 2023 21:04

NullVoxPopuli approved these changes Dec 5, 2023

View reviewed changes

ef4 merged commit 82a1b20 into main Dec 11, 2023
18 checks passed

ef4 deleted the direct-eval-template-compiler branch December 11, 2023 12:42

ef4 mentioned this pull request Dec 12, 2023

[BUGFIX beta] internal templates should be strictMode #20603

Merged

ef4 mentioned this pull request Dec 15, 2023

[BUGFIX beta] move babel-plugin-ember-template-compilation to dependencies #20605

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reform internal use of template compiler #20587

Reform internal use of template compiler #20587

ef4 commented Dec 1, 2023 •

edited

Loading

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 Dec 1, 2023

ef4 commented Dec 1, 2023

NullVoxPopuli left a comment

		@@ -0,0 +1,2 @@
		// this file gets replaced with the real value during the build
		export default 'VERSION_GOES_HERE';

		@@ -0,0 +1,2 @@
		# this tells pnpm we are a separate project with a separate install, not part of
		# the monorepo's workspaces.

Reform internal use of template compiler #20587

Reform internal use of template compiler #20587

Conversation

ef4 commented Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ef4 commented Dec 1, 2023

NullVoxPopuli left a comment

Choose a reason for hiding this comment

ef4 commented Dec 1, 2023 •

edited

Loading